Skip to content

fix: scale raw ADC values in CSV and JSON SD card parsers#152

Merged
tylerkron merged 2 commits intomainfrom
fix/csv-json-adc-scaling
Mar 20, 2026
Merged

fix: scale raw ADC values in CSV and JSON SD card parsers#152
tylerkron merged 2 commits intomainfrom
fix/csv-json-adc-scaling

Conversation

@tylerkron
Copy link
Copy Markdown
Contributor

The firmware writes raw ADC integer counts (not voltages) to CSV and JSON log files. The CSV parser was passing these through as-is, causing values like 22 ADC counts to display as 22V instead of ~0.003V.

Changes:

  • Add ScaleRawAnalogValues to both CSV and JSON parsers, matching the protobuf parser's formula: (raw/resolution * portRange * calM + calB) * scaleM
  • Parse CSV column headers to distinguish ain* (analog) from dio* (digital) columns — previously the dio pair was treated as analog
  • Add tests for scaling with/without calibration config and DIO handling

The firmware writes raw ADC integer counts (not voltages) to CSV and
JSON log files. The CSV parser was passing these through as-is, causing
values like 22 ADC counts to display as 22V instead of ~0.003V.

Changes:
- Add ScaleRawAnalogValues to both CSV and JSON parsers, matching the
  protobuf parser's formula: (raw/resolution * portRange * calM + calB) * scaleM
- Parse CSV column headers to distinguish ain* (analog) from dio*
  (digital) columns — previously the dio pair was treated as analog
- Add tests for scaling with/without calibration config and DIO handling

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: a0fb8798807b
@tylerkron tylerkron requested a review from a team as a code owner March 20, 2026 03:27
@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Scale raw ADC values in CSV and JSON SD card parsers

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Scale raw ADC values in CSV and JSON parsers using calibration formula
• Parse CSV column headers to distinguish analog (ain*) from digital (dio*) columns
• Add comprehensive tests for ADC scaling with/without calibration config
• Fix digital data handling in CSV parser (previously treated as analog)
Diagram
flowchart LR
  Raw["Raw ADC Values<br/>from CSV/JSON"]
  Parse["Parse Column Layout<br/>ain* vs dio*"]
  Scale["Scale Formula<br/>raw/resolution * range * calM + calB * scaleM"]
  Output["Scaled Voltage Values<br/>+ Digital Data"]
  Raw --> Parse
  Parse --> Scale
  Scale --> Output
Loading

Grey Divider

File Changes

1. src/Daqifi.Core.Tests/Device/SdCard/SdCardCsvFileParserTests.cs 🧪 Tests +200/-3

Add comprehensive ADC scaling tests for CSV parser

• Renamed test method to clarify DIO column behavior
• Added 4 new test cases for ADC scaling with calibration config
• Added tests for scaling with/without calibration and full formula validation
• Added tests for DIO column parsing and analog-only scaling
• Added test for ain column header parsing

src/Daqifi.Core.Tests/Device/SdCard/SdCardCsvFileParserTests.cs


2. src/Daqifi.Core.Tests/Device/SdCard/SdCardJsonFileParserTests.cs 🧪 Tests +64/-0

Add ADC scaling tests for JSON parser

• Added 2 new test cases for ADC scaling in JSON parser
• Test scaling with calibration config (raw 22 → ~0.00336V)
• Test raw value passthrough when no calibration config provided

src/Daqifi.Core.Tests/Device/SdCard/SdCardJsonFileParserTests.cs


3. src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs ✨ Enhancement +122/-24

Implement ADC scaling and DIO column parsing in CSV parser

• Added CsvColumnLayout record to track analog pair count and digital pair presence
• Modified ParseHeader() to return both config and column layout, distinguishing ain* from dio*
 columns
• Updated TryParseCsvDataRow() to parse digital data separately from analog values
• Added ScaleRawAnalogValues() method implementing formula: (raw/resolution * portRange * calM +
 calB) * scaleM
• Updated ParseCsvLines() to apply scaling and pass digital data to log entries
• Updated documentation to reflect ain* column naming and raw ADC value scaling

src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs


View more (1)
4. src/Daqifi.Core/Device/SdCard/SdCardJsonFileParser.cs ✨ Enhancement +39/-1

Implement ADC scaling in JSON parser

• Added ScaleRawAnalogValues() method with same formula as CSV parser
• Updated ParseJsonLines() to apply scaling to raw analog values
• Maintains consistency with CSV parser scaling implementation

src/Daqifi.Core/Device/SdCard/SdCardJsonFileParser.cs


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Mar 20, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. Skips dio-only rows🐞 Bug ✓ Correctness
Description
SdCardCsvFileParser.TryParseCsvDataRow returns null when no analog pairs are parsed, even if a DIO
pair is present, so CSV logs containing only dio_ts/dio_val will yield zero samples. This causes
silent data loss for digital-only configurations/files.
Code

src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[R369-384]

+            // Parse digital I/O pair if present (last pair)
+            uint digitalData = 0;
+            if (layout.HasDigitalPair && totalPairs > analogPairCount)
+            {
+                var dioIndex = analogPairCount;
+                var dioValCol = columns[dioIndex * 2 + 1];
+                if (uint.TryParse(dioValCol, NumberStyles.None, CultureInfo.InvariantCulture, out var dioVal))
+                {
+                    digitalData = dioVal;
+                }
+            }
+
+            if (perChannelTimestamps.Count == 0)
+            {
+                return null;
+            }
Evidence
TryParseCsvDataRow only populates perChannelTimestamps while parsing analog pairs; when
analogPairCount is 0, perChannelTimestamps remains empty, digitalData can still be parsed, but the
method returns null and the caller skips the line. ParseHeader can produce a layout where
AnalogPairCount==0 and HasDigitalPair==true when the header indicates only a dio pair.

src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[136-223]
src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[341-388]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TryParseCsvDataRow` drops rows when `analogPairCount == 0` (no analog channel pairs), even if a valid `dio_ts,dio_val` pair exists. This makes CSV files that contain only digital samples (or configurations with 0 analog channels) return no samples.
## Issue Context
- `TryParseCsvDataRow` only adds timestamps while parsing analog pairs.
- It parses `digitalData` from the presumed dio pair, then returns `null` when `perChannelTimestamps.Count == 0`.
- The caller treats `null` as a malformed row and skips it.
## Fix Focus Areas
- src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[329-388]
- src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[136-223]
## Implementation notes
- If `analogPairCount == 0` and `layout.HasDigitalPair` is true, parse `dio_ts` and use it as `rowTimestamp` for time reconstruction.
- Return an empty analog list and an empty (or null-equivalent) analog timestamp list rather than `null` so the row is emitted with `DigitalData` populated.
- Add/adjust a unit test for a `dio_ts,dio_val`-only CSV header + data row to prevent regression.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. DIO column mis-indexed 🐞 Bug ✓ Correctness
Description
SdCardCsvFileParser assumes the DIO pair is the last column pair by using dioIndex =
analogPairCount, but ParseHeader only records a boolean HasDigitalPair and does not enforce/record
the DIO pair position. If a CSV’s DIO pair is not last, digital values (and potentially analog
values) will be read from the wrong columns without errors.
Code

src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[R369-379]

+            // Parse digital I/O pair if present (last pair)
+            uint digitalData = 0;
+            if (layout.HasDigitalPair && totalPairs > analogPairCount)
+            {
+                var dioIndex = analogPairCount;
+                var dioValCol = columns[dioIndex * 2 + 1];
+                if (uint.TryParse(dioValCol, NumberStyles.None, CultureInfo.InvariantCulture, out var dioVal))
+                {
+                    digitalData = dioVal;
+                }
+            }
Evidence
ParseHeader sets HasDigitalPair=true upon seeing any timestamp column starting with 'dio', but does
not record the index of that pair; TryParseCsvDataRow then assumes the dio pair sits exactly after
the analog pairs (i.e., last) by using dioIndex = analogPairCount. This only works when the dio
pair is last and all analog pairs precede it.

src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[179-203]
src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[341-379]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The CSV parser detects the presence of a DIO pair, but does not record its position. Row parsing then assumes the DIO pair is the last pair (`dioIndex = analogPairCount`), which will mis-parse files where the DIO columns are not last.
## Issue Context
- `ParseHeader` currently outputs `CsvColumnLayout(int AnalogPairCount, bool HasDigitalPair)`.
- `TryParseCsvDataRow` uses `dioIndex = analogPairCount` and reads `columns[dioIndex * 2 + 1]`.
## Fix Focus Areas
- src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[126-223]
- src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[329-388]
## Implementation notes
- Change `CsvColumnLayout` to store an optional `DigitalPairIndex` (e.g., `int? DigitalPairIndex` or `int DigitalPairIndex = -1`).
- In `ParseHeader`, record the pair index where the &amp;#x27;dio&amp;#x27; columns appear and optionally validate that there is at most one.
- In `TryParseCsvDataRow`, parse analog pairs by skipping the digital pair index rather than assuming digital is last.
- Add a targeted unit test with a non-last DIO pair ordering to ensure correct behavior (or explicitly enforce and fail fast if non-last is unsupported).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Use the dio timestamp as the row timestamp when no analog channels are
present, instead of returning null and silently dropping the row.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: a0fb8798807b
@tylerkron tylerkron merged commit d4be3aa into main Mar 20, 2026
1 check passed
@tylerkron tylerkron deleted the fix/csv-json-adc-scaling branch March 20, 2026 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant